Skip to content

TA-4530: Implement secure secret storage#323

Merged
Laberion Ajvazi (LaberionAjvazi) merged 10 commits intomasterfrom
LaberionAjvazi/ta-4530
Mar 19, 2026
Merged

TA-4530: Implement secure secret storage#323
Laberion Ajvazi (LaberionAjvazi) merged 10 commits intomasterfrom
LaberionAjvazi/ta-4530

Conversation

@LaberionAjvazi
Copy link
Contributor

@LaberionAjvazi Laberion Ajvazi (LaberionAjvazi) commented Mar 17, 2026

Description

Implemented secure secret storage using @github/keytar (the actively maintained fork of atom/node-keytar) with fallback to the current plain text storage for cases where storing the secrets in the system keychain fails or is not available (e.g., on Linux due to libsecret dependency).

A warning will be shown to users when creating or using a profile with insecure secret storage.

Changes from the original PR #301:

  • Replaced the archived keytar (atom/node-keytar v7.9.0) with @github/keytar v7.10.6 (pinned version, no caret) — same API, actively maintained
  • Added content-cli profile secure <profile> command to migrate existing plaintext profiles to secure keychain storage without re-creating them (addresses Meris's review comment on the closed PR)
  • Added @aocelo and @siavash-celonis as CODEOWNERS for package.json and yarn.lock
  • Rebased onto current master (resolved conflicts with dependabot bumps, TA-4814, TA-4654)

Summary of changes

  • New SecureSecretStorageService using @github/keytar to save/retrieve apiToken, clientSecret, and refreshToken under celonis-content-cli:<profile-name>
  • Profile model updated (ProfileSecrets, secretsStoredSecurely flag); storeProfile now async, strips secrets when saved securely; findProfile loads secrets from keychain when flagged
  • ProfileCommandService/ProfileService updated to await storeProfile and persist refreshed tokens securely
  • New profile secure <profile> CLI command for migrating existing plaintext profiles to secure storage
  • Documentation expanded with security behavior, storage locations, best practices, and migration instructions

Relevant links

Checklist

  • I have self-reviewed this PR
  • I have tested the change and proved that it works in different scenarios
  • I have updated docs if needed

… secure command

- Swap archived keytar (atom/node-keytar 7.9.0) for maintained fork
  (@github/keytar 7.10.6, pinned version)
- Add `content-cli profile secure <profile>` command to migrate
  existing plaintext profiles to system keychain storage
- Add @aocelo and @siavash-celonis as CODEOWNERS for package.json
  and yarn.lock

Includes-AI-Code: true
Made-with: Cursor
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Wrap the keytar findCredentials call in getSecrets with try-catch so
that keychain errors (locked, permission denied) return undefined
instead of propagating to the caller with a generic error message.

Includes-AI-Code: true
Made-with: Cursor
@sonarqubecloud
Copy link

@LaberionAjvazi Laberion Ajvazi (LaberionAjvazi) merged commit dc70c25 into master Mar 19, 2026
7 checks passed
@LaberionAjvazi Laberion Ajvazi (LaberionAjvazi) deleted the LaberionAjvazi/ta-4530 branch March 19, 2026 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants